Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provenance v1.0: initial draft #525

Merged
merged 47 commits into from
Jan 20, 2023

Conversation

MarkLodato
Copy link
Member

@MarkLodato MarkLodato commented Nov 1, 2022

This is an initial draft v1.0 for the provenance format. It attempts to address the major sources of confusion with the current format (v0.2), particularly the separation of concerns between top-level external inputs, resolved dependencies, and per-run details.

There are still many TODOs. I hope to get this merged sooner rather than later so that we can get feedback on the schema and substance, then send further PRs to address the remaining TODOs.

(I'll rewrite the history before merging so that it's not full of all my "WIP" commits.)

Preview: https://deploy-preview-525--slsa.netlify.app/provenance/v1/

Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Move all top-level inputs to their own field so that it is clear what
must be verified. Move resolvedDependencies outside since it's not
top-level.

Create new buildDependencies containing resolvedDependencies and
environment. This way buildDefinition is complete.

Revert field names to their v0.2 names, since the name change isn't
worth the switch.

Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
@netlify
Copy link

netlify bot commented Nov 1, 2022

Deploy Preview for slsa ready!

Name Link
🔨 Latest commit 4210074
🔍 Latest deploy log https://app.netlify.com/sites/slsa/deploys/63cad744a0262e0008a3f816
😎 Deploy Preview https://deploy-preview-525--slsa.netlify.app/provenance/v1
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Signed-off-by: Mark Lodato <lodato@google.com>
Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really good. I did a first pass review and added some minor observations and comments.

docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
Signed-off-by: Mark Lodato <lodato@google.com>
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Also move to a subdirectory (1) since we now have multiple files and (2)
to make relative includes work correclty whether or not the URL has a
trailing slash.

Signed-off-by: Mark Lodato <lodato@google.com>
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
// TODO: Flesh out this model more.
//
// OPTIONAL.
repeated ArtifactReference builderDependencies = 3;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A CI pipeline's binaries could be categorized in 2 ways.

  1. The list of orchestrator tools like Tekton Pipeline, TektonChains, TektonTriggers...
  2. The list of actual tools which make the build like gcc.
    Should this tools from only from (1) or even (2).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only be (1). (2) should go in resolvedDependencies. Do you have a suggestion to make that more clear?

docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Copy link
Member Author

@MarkLodato MarkLodato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK everyone, thanks for the very helpful feedback! I'm hoping to get this merged this week, so if you could review end-of-day Friday (US Eastern), that would be appreciated!

There are still many TODOs and outstanding issues, particularly on the design of the parameters. The intention is that we submit this and iterate. By having it checked in, we can more easily get feedback from potential implementers.

Preview: https://deploy-preview-525--slsa.netlify.app/provenance/v1/

NOTES:

  1. I aggressively marked GitHub comment threads as "resolved" in order to reduce noise. I did my best to try to capture the intent either as a TODO or actually implementing it. Feel free to reopen threads, though the UI doesn't make that particularly easy...

  2. I reorganized a bit and changed around the documentation format. Please take a look. I'm hoping this new format is now easier to follow. (The reason for moving under a directory is to make it easier to have related files, and the reason for dropping the ".0" from the URL is that is what our documentation says we would do, since minor version bumps are backwards compatible.)

Thanks all!

docs/github-actions-workflow/v0.1/example.json Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
docs/_data/versions.yml Outdated Show resolved Hide resolved
docs/github-actions-workflow/v0.1/example.json Outdated Show resolved Hide resolved
"digest": { "sha1": "c27d339ee6075c1f744c5d4b200f7901aad2c369" }
}
},
"workflow_path": { "value": ".github/workflow/release.yml" }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this thread OK to resolve, or is there a specific request here (hard to follow thread)?

docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
// TODO: Flesh out this model more.
//
// OPTIONAL.
repeated ArtifactReference builderDependencies = 3;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should only be (1). (2) should go in resolvedDependencies. Do you have a suggestion to make that more clear?

docs/provenance/v1.0.proto Outdated Show resolved Hide resolved
docs/provenance/v1.0.md Outdated Show resolved Hide resolved
Signed-off-by: Mark Lodato <lodato@google.com>
Signed-off-by: Mark Lodato <lodato@google.com>
Copy link
Member

@trishankatdatadog trishankatdatadog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks very much! Could we please ask another CI provider (e.g., GitLab) to contribute their own Build Type to make sure that we have designed a general enough Build Provenance?

Signed-off-by: Mark Lodato <lodato@google.com>
@MarkLodato
Copy link
Member Author

Looks good to me, thanks very much! Could we please ask another CI provider (e.g., GitLab) to contribute their own Build Type to make sure that we have designed a general enough Build Provenance?

Good point. Added TODO noting that it's a blocker for calling it stable.

@trishankatdatadog
Copy link
Member

Good point. Added TODO noting that it's a blocker for calling it stable.

Approved, but shouldn't hang on my opinion alone 🙂

Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fantastic evolution of the provenance format. Thanks for calling out the bits that would especially benefit from feedback with the "RFC" markers.

I fully support merging this as-is and iterating in tree. I have made a few minor suggestions to fix some of the links, otherwise LGTM.

docs/provenance/v1/provenance.cue Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
docs/provenance/v1/index.md Outdated Show resolved Hide resolved
Signed-off-by: Mark Lodato <lodato@google.com>
@MarkLodato
Copy link
Member Author

Thanks. I fixed all those issues, I think. I'll do a "squash and merge", but I'll leave this open for a bit for you to verify if you have time.

Copy link
Member

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

docs/provenance/v1/index.md Outdated Show resolved Hide resolved
Signed-off-by: Mark Lodato <lodato@google.com>
@MarkLodato MarkLodato merged commit e5cbd09 into slsa-framework:main Jan 20, 2023
@MarkLodato MarkLodato deleted the provenance-refactor branch January 20, 2023 18:09
@MarkLodato
Copy link
Member Author

Thanks all! This is now squashed and merged, but please continue to provide feedback and suggestions.

"id": "https://github.com/slsa-framework/slsa-github-generator/.github/workflows/builder_go_slsa3.yml@refs/tags/v0.0.1"
},
"metadata": {
"invocationId": "https://github.com/octocat/hello-world/actions/runs/1536140711/attempts/1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love having the full URL in here!

chtiangg pushed a commit to chtiangg/slsa that referenced this pull request May 22, 2023
This is an initial draft v1.0 for the provenance format. It attempts to address
the major sources of confusion with v0.2, particularly the separation of
concerns between top-level external inputs, resolved dependencies, and per-run
details.

There are still many outstanding TODOs and unresolved design decisions. The idea
is to submit this now so that we can more easily get feedback on the schema and
substance, then send further PRs to address the remaining TODOs and RFCs.

Signed-off-by: Mark Lodato <lodato@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.